-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[MLIR][OpenMP] Add OMP Mapper field to MapInfoOp #120994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-openmp Author: Akash Banerjee (TIFitis) ChangesThis patch adds the mapper field to the omp.map.info op. Full diff: https://github.com/llvm/llvm-project/pull/120994.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 881f28d42e6312..7cc2c8fa8ce1e1 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -1000,6 +1000,7 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> {
OptionalAttr<IndexListArrayAttr>:$members_index,
Variadic<OpenMP_MapBoundsType>:$bounds, /* rank-0 to rank-{n-1} */
OptionalAttr<UI64Attr>:$map_type,
+ OptionalAttr<FlatSymbolRefAttr>:$mapper_id,
OptionalAttr<VariableCaptureKindAttr>:$map_capture_type,
OptionalAttr<StrAttr>:$name,
DefaultValuedAttr<BoolAttr, "false">:$partial_map);
@@ -1064,6 +1065,7 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> {
`var_ptr` `(` $var_ptr `:` type($var_ptr) `,` $var_type `)`
oilist(
`var_ptr_ptr` `(` $var_ptr_ptr `:` type($var_ptr_ptr) `)`
+ | `mapper` `(` $mapper_id `)`
| `map_clauses` `(` custom<MapClause>($map_type) `)`
| `capture` `(` custom<CaptureType>($map_capture_type) `)`
| `members` `(` $members `:` custom<MembersIndex>($members_index) `:` type($members) `)`
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index b15d6ed15244ed..92233654ba1ddc 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1592,7 +1592,7 @@ static LogicalResult verifyMapClause(Operation *op, OperandRange mapVars) {
to ? updateToVars.insert(updateVar) : updateFromVars.insert(updateVar);
}
- } else {
+ } else if (!isa<DeclareMapperInfoOp>(op)) {
emitError(op->getLoc(), "map argument is not a map entry operation");
}
}
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index a167c8bd1abbf9..97ef132f6dfa53 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -2523,13 +2523,13 @@ func.func @omp_targets_with_map_bounds(%arg0: !llvm.ptr, %arg1: !llvm.ptr) -> ()
// CHECK: %[[C_12:.*]] = llvm.mlir.constant(2 : index) : i64
// CHECK: %[[C_13:.*]] = llvm.mlir.constant(2 : index) : i64
// CHECK: %[[BOUNDS1:.*]] = omp.map.bounds lower_bound(%[[C_11]] : i64) upper_bound(%[[C_10]] : i64) stride(%[[C_12]] : i64) start_idx(%[[C_13]] : i64)
- // CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, !llvm.array<10 x i32>) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%[[BOUNDS1]]) -> !llvm.ptr {name = ""}
+ // CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, !llvm.array<10 x i32>) mapper(@my_mapper) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%[[BOUNDS1]]) -> !llvm.ptr {name = ""}
%6 = llvm.mlir.constant(9 : index) : i64
%7 = llvm.mlir.constant(1 : index) : i64
%8 = llvm.mlir.constant(2 : index) : i64
%9 = llvm.mlir.constant(2 : index) : i64
%10 = omp.map.bounds lower_bound(%7 : i64) upper_bound(%6 : i64) stride(%8 : i64) start_idx(%9 : i64)
- %mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, !llvm.array<10 x i32>) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%10) -> !llvm.ptr {name = ""}
+ %mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, !llvm.array<10 x i32>) mapper(@my_mapper) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%10) -> !llvm.ptr {name = ""}
// CHECK: omp.target map_entries(%[[MAP0]] -> {{.*}}, %[[MAP1]] -> {{.*}} : !llvm.ptr, !llvm.ptr)
omp.target map_entries(%mapv1 -> %arg2, %mapv2 -> %arg3 : !llvm.ptr, !llvm.ptr) {
|
|
@llvm/pr-subscribers-mlir Author: Akash Banerjee (TIFitis) ChangesThis patch adds the mapper field to the omp.map.info op. Full diff: https://github.com/llvm/llvm-project/pull/120994.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 881f28d42e6312..7cc2c8fa8ce1e1 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -1000,6 +1000,7 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> {
OptionalAttr<IndexListArrayAttr>:$members_index,
Variadic<OpenMP_MapBoundsType>:$bounds, /* rank-0 to rank-{n-1} */
OptionalAttr<UI64Attr>:$map_type,
+ OptionalAttr<FlatSymbolRefAttr>:$mapper_id,
OptionalAttr<VariableCaptureKindAttr>:$map_capture_type,
OptionalAttr<StrAttr>:$name,
DefaultValuedAttr<BoolAttr, "false">:$partial_map);
@@ -1064,6 +1065,7 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> {
`var_ptr` `(` $var_ptr `:` type($var_ptr) `,` $var_type `)`
oilist(
`var_ptr_ptr` `(` $var_ptr_ptr `:` type($var_ptr_ptr) `)`
+ | `mapper` `(` $mapper_id `)`
| `map_clauses` `(` custom<MapClause>($map_type) `)`
| `capture` `(` custom<CaptureType>($map_capture_type) `)`
| `members` `(` $members `:` custom<MembersIndex>($members_index) `:` type($members) `)`
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index b15d6ed15244ed..92233654ba1ddc 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1592,7 +1592,7 @@ static LogicalResult verifyMapClause(Operation *op, OperandRange mapVars) {
to ? updateToVars.insert(updateVar) : updateFromVars.insert(updateVar);
}
- } else {
+ } else if (!isa<DeclareMapperInfoOp>(op)) {
emitError(op->getLoc(), "map argument is not a map entry operation");
}
}
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index a167c8bd1abbf9..97ef132f6dfa53 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -2523,13 +2523,13 @@ func.func @omp_targets_with_map_bounds(%arg0: !llvm.ptr, %arg1: !llvm.ptr) -> ()
// CHECK: %[[C_12:.*]] = llvm.mlir.constant(2 : index) : i64
// CHECK: %[[C_13:.*]] = llvm.mlir.constant(2 : index) : i64
// CHECK: %[[BOUNDS1:.*]] = omp.map.bounds lower_bound(%[[C_11]] : i64) upper_bound(%[[C_10]] : i64) stride(%[[C_12]] : i64) start_idx(%[[C_13]] : i64)
- // CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, !llvm.array<10 x i32>) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%[[BOUNDS1]]) -> !llvm.ptr {name = ""}
+ // CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, !llvm.array<10 x i32>) mapper(@my_mapper) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%[[BOUNDS1]]) -> !llvm.ptr {name = ""}
%6 = llvm.mlir.constant(9 : index) : i64
%7 = llvm.mlir.constant(1 : index) : i64
%8 = llvm.mlir.constant(2 : index) : i64
%9 = llvm.mlir.constant(2 : index) : i64
%10 = omp.map.bounds lower_bound(%7 : i64) upper_bound(%6 : i64) stride(%8 : i64) start_idx(%9 : i64)
- %mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, !llvm.array<10 x i32>) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%10) -> !llvm.ptr {name = ""}
+ %mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, !llvm.array<10 x i32>) mapper(@my_mapper) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%10) -> !llvm.ptr {name = ""}
// CHECK: omp.target map_entries(%[[MAP0]] -> {{.*}}, %[[MAP1]] -> {{.*}} : !llvm.ptr, !llvm.ptr)
omp.target map_entries(%mapv1 -> %arg2, %mapv2 -> %arg3 : !llvm.ptr, !llvm.ptr) {
|
|
@llvm/pr-subscribers-mlir-openmp Author: Akash Banerjee (TIFitis) ChangesThis patch adds the mapper field to the omp.map.info op. Full diff: https://github.com/llvm/llvm-project/pull/120994.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 881f28d42e6312..7cc2c8fa8ce1e1 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -1000,6 +1000,7 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> {
OptionalAttr<IndexListArrayAttr>:$members_index,
Variadic<OpenMP_MapBoundsType>:$bounds, /* rank-0 to rank-{n-1} */
OptionalAttr<UI64Attr>:$map_type,
+ OptionalAttr<FlatSymbolRefAttr>:$mapper_id,
OptionalAttr<VariableCaptureKindAttr>:$map_capture_type,
OptionalAttr<StrAttr>:$name,
DefaultValuedAttr<BoolAttr, "false">:$partial_map);
@@ -1064,6 +1065,7 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> {
`var_ptr` `(` $var_ptr `:` type($var_ptr) `,` $var_type `)`
oilist(
`var_ptr_ptr` `(` $var_ptr_ptr `:` type($var_ptr_ptr) `)`
+ | `mapper` `(` $mapper_id `)`
| `map_clauses` `(` custom<MapClause>($map_type) `)`
| `capture` `(` custom<CaptureType>($map_capture_type) `)`
| `members` `(` $members `:` custom<MembersIndex>($members_index) `:` type($members) `)`
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index b15d6ed15244ed..92233654ba1ddc 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1592,7 +1592,7 @@ static LogicalResult verifyMapClause(Operation *op, OperandRange mapVars) {
to ? updateToVars.insert(updateVar) : updateFromVars.insert(updateVar);
}
- } else {
+ } else if (!isa<DeclareMapperInfoOp>(op)) {
emitError(op->getLoc(), "map argument is not a map entry operation");
}
}
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index a167c8bd1abbf9..97ef132f6dfa53 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -2523,13 +2523,13 @@ func.func @omp_targets_with_map_bounds(%arg0: !llvm.ptr, %arg1: !llvm.ptr) -> ()
// CHECK: %[[C_12:.*]] = llvm.mlir.constant(2 : index) : i64
// CHECK: %[[C_13:.*]] = llvm.mlir.constant(2 : index) : i64
// CHECK: %[[BOUNDS1:.*]] = omp.map.bounds lower_bound(%[[C_11]] : i64) upper_bound(%[[C_10]] : i64) stride(%[[C_12]] : i64) start_idx(%[[C_13]] : i64)
- // CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, !llvm.array<10 x i32>) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%[[BOUNDS1]]) -> !llvm.ptr {name = ""}
+ // CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, !llvm.array<10 x i32>) mapper(@my_mapper) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%[[BOUNDS1]]) -> !llvm.ptr {name = ""}
%6 = llvm.mlir.constant(9 : index) : i64
%7 = llvm.mlir.constant(1 : index) : i64
%8 = llvm.mlir.constant(2 : index) : i64
%9 = llvm.mlir.constant(2 : index) : i64
%10 = omp.map.bounds lower_bound(%7 : i64) upper_bound(%6 : i64) stride(%8 : i64) start_idx(%9 : i64)
- %mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, !llvm.array<10 x i32>) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%10) -> !llvm.ptr {name = ""}
+ %mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, !llvm.array<10 x i32>) mapper(@my_mapper) map_clauses(exit_release_or_enter_alloc) capture(ByCopy) bounds(%10) -> !llvm.ptr {name = ""}
// CHECK: omp.target map_entries(%[[MAP0]] -> {{.*}}, %[[MAP1]] -> {{.*}} : !llvm.ptr, !llvm.ptr)
omp.target map_entries(%mapv1 -> %arg2, %mapv2 -> %arg3 : !llvm.ptr, !llvm.ptr) {
|
21178b0 to
0b54caf
Compare
0b54caf to
002d715
Compare
ergawy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| OptionalAttr<IndexListArrayAttr>:$members_index, | ||
| Variadic<OpenMP_MapBoundsType>:$bounds, /* rank-0 to rank-{n-1} */ | ||
| OptionalAttr<UI64Attr>:$map_type, | ||
| OptionalAttr<FlatSymbolRefAttr>:$mapper_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a description for this argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it can only be the name of a declare mapper then it might be good to add that to the verifier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, I was on vacation.
I've updated the description and also added a verifier check.
skatrak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
|
One small comment, though. Please trigger a not-yet-implemented error when translating all operations taking |
Is that necessary given I have the entire implementation in place? I am planning on merging the entire series in one go. |
49305ab to
7fb1429
Compare
5ca5a1f to
86513d3
Compare
If you plan on merging the whole stack at once and if it will add support for declare mappers to all operations that take map clauses, then I agree it would be fine to skip adding any not-yet-implemented errors here. |
7fb1429 to
3c15b95
Compare
86513d3 to
e93c05d
Compare
3c15b95 to
5d7a402
Compare
e93c05d to
e905951
Compare
5d7a402 to
aa7f4aa
Compare
e905951 to
19e22eb
Compare
aa7f4aa to
afb17c9
Compare
19e22eb to
04831ef
Compare
04831ef to
03838f7
Compare
This patch adds the mapper field to the omp.map.info op. Depends on llvm#117046.
…clause (llvm#121001) Add Lowering support for OpenMP mapper field in mapInfoOp. NOTE: This patch only supports explicit mapper lowering. I'll add a separate PR soon which handles implicit default mapper recognition. Depends on llvm#120994.
land and revert: 785a5b4 [MLIR][OpenMP] Add LLVM translation support for OpenMP UserDefinedMappers (llvm#124746) d6ab12c [MLIR][OpenMP] Add conversion support from FIR to LLVM Dialect for OMP DeclareMapper (llvm#121005) 886b2ed [MLIR][OpenMP] Add Lowering support for OpenMP custom mappers in map clause (llvm#121001) ee17955 [MLIR][OpenMP] Add OMP Mapper field to MapInfoOp (llvm#120994)
This patch adds the mapper field to the omp.map.info op. Depends on llvm#117046.
…clause (llvm#121001) Add Lowering support for OpenMP mapper field in mapInfoOp. NOTE: This patch only supports explicit mapper lowering. I'll add a separate PR soon which handles implicit default mapper recognition. Depends on llvm#120994.
This patch adds the mapper field to the omp.map.info op. Depends on llvm#117046.
…clause (llvm#121001) Add Lowering support for OpenMP mapper field in mapInfoOp. NOTE: This patch only supports explicit mapper lowering. I'll add a separate PR soon which handles implicit default mapper recognition. Depends on llvm#120994.
This patch adds the mapper field to the omp.map.info op.
Depends on #117046.